-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testnets #128
Testnets #128
Conversation
WalkthroughThe changes involve updates to several components within the application. The Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for proof-of-humanity-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (4)
src/components/Progress.tsx (1)
8-8
: LGTM! Consider adding ARIA attributes for better accessibility.The text-sm class addition maintains consistent typography. However, for better accessibility, consider adding appropriate ARIA attributes.
- <div className="mb-1 text-sm">{label}</div> + <div className="mb-1 text-sm" aria-label={label}>{label}</div> <div className="mb-4 h-2 w-full rounded-full bg-slate-200"> <div className="h-full rounded-full bg-lime-500" + role="progressbar" + aria-valuenow={value} + aria-valuemin={0} + aria-valuemax={100} style={{ width: `${value}%` }} /> </div>src/components/BulletedNumber.tsx (1)
3-6
: Consider adding prop validation for the number value.The interface looks good, but consider adding validation or constraints for the
number
prop to ensure it's a positive integer, as this appears to be used for step numbering.interface BulletedNumberProps { - number: number; + number: number; // Should be a positive integer current?: boolean; + + // Consider adding validation + validate?: { + min?: number; + max?: number; + }; }src/app/[pohid]/[chain]/[request]/Appeal.tsx (2)
87-121
: Consider adding ARIA labels for better accessibilityThe UI restructuring looks great with improved visual hierarchy and user feedback. Consider adding aria-labels to the funding progress bar for better accessibility.
<Progress value={valueProgress} label={`${formatEth(requesterFunds)} ${unit} out of ${formatEth(appealCost)} ${unit}`} + aria-label={`Funding progress: ${valueProgress}% complete`} />
347-355
: Add security attributes to external linkThe external link to Kleros should include security-related attributes.
<ExternalLink className="text-orange mx-2 flex flex-row flex-wrap justify-end gap-x-[8px] text-sm font-semibold leading-none hover:text-orange-500 md:gap-2 lg:gap-3" href={`https://resolve.kleros.io/cases/${currentChallenge.disputeId}`} + rel="noopener noreferrer" + target="_blank" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
public/logo/exclamation.svg
is excluded by!**/*.svg
📒 Files selected for processing (6)
src/app/[pohid]/[chain]/[request]/ActionBar.tsx
(1 hunks)src/app/[pohid]/[chain]/[request]/Appeal.tsx
(7 hunks)src/app/[pohid]/[chain]/[request]/Info.tsx
(1 hunks)src/app/[pohid]/[chain]/[request]/page.tsx
(2 hunks)src/components/BulletedNumber.tsx
(1 hunks)src/components/Progress.tsx
(1 hunks)
🔇 Additional comments (10)
src/app/[pohid]/[chain]/[request]/Info.tsx (3)
3-3
: LGTM! Import statement cleanup looks good.
9-9
: LGTM! Interface update is well-typed.
12-12
: LGTM! Component signature properly reflects interface changes.
src/components/BulletedNumber.tsx (2)
1-2
: LGTM! Correct usage of the client directive.
The "use client" directive is appropriately placed at the top of the file, marking this as a Client Component for Next.js.
30-30
: LGTM! Correct export statement.
The default export is appropriately used for this single component.
src/app/[pohid]/[chain]/[request]/Appeal.tsx (3)
4-6
: LGTM: Import statements are well-organized
The new imports are properly organized and necessary for the enhanced UI components.
Also applies to: 23-23, 26-26
138-139
: LGTM: Props interface update is well-typed
The new boolean props revocation
and expired
are properly typed and consistently used in the component parameters.
Also applies to: 153-154
313-399
: LGTM: Well-structured step-by-step UI
The new step-by-step UI with BulletedNumber
components and clear explanations greatly improves user understanding of the appeal process. The conditional rendering based on revocation status is handled appropriately.
src/app/[pohid]/[chain]/[request]/page.tsx (1)
364-370
: LGTM: Info component usage is clear and well-structured.
The Info component's implementation with the explicit label prop and request count calculation is clean and maintainable.
src/app/[pohid]/[chain]/[request]/ActionBar.tsx (1)
497-498
: Props revocation
and expired
passed to Appeal
component
The revocation
and expired
props are correctly passed to the Appeal
component to handle conditional rendering based on these states.
<span className="flex cursor-pointer gap-x-[4px] text-slate-500 hover:text-slate-700"> | ||
{label} | ||
<InfoIcon className="h-6 w-6 stroke-slate-500 stroke-2 hover:stroke-slate-700" /> | ||
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider improving accessibility of the modal trigger.
The current implementation has accessibility limitations:
- Using a
span
withcursor-pointer
for an interactive element instead of a semanticbutton
- Missing keyboard interaction support
- No ARIA labels for screen readers
Consider applying these improvements:
- <span className="flex cursor-pointer gap-x-[4px] text-slate-500 hover:text-slate-700">
+ <button
+ type="button"
+ className="flex items-center gap-x-[4px] text-slate-500 hover:text-slate-700"
+ aria-label={`Information about ${label}`}
+ >
{label}
<InfoIcon className="h-6 w-6 stroke-slate-500 stroke-2 hover:stroke-slate-700" />
- </span>
+ </button>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<span className="flex cursor-pointer gap-x-[4px] text-slate-500 hover:text-slate-700"> | |
{label} | |
<InfoIcon className="h-6 w-6 stroke-slate-500 stroke-2 hover:stroke-slate-700" /> | |
</span> | |
<button | |
type="button" | |
className="flex items-center gap-x-[4px] text-slate-500 hover:text-slate-700" | |
aria-label={`Information about ${label}`} | |
> | |
{label} | |
<InfoIcon className="h-6 w-6 stroke-slate-500 stroke-2 hover:stroke-slate-700" /> | |
</button> |
<div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"> | ||
<span className="absolute inset-0 flex items-center justify-center"> | ||
<span className="py-0.25 rounded-full px-0.5 text-sm leading-none text-white"> | ||
{number} | ||
</span> | ||
</span> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider extracting gradient colors to CSS variables.
The gradient colors are hardcoded. Consider moving them to CSS variables for better maintainability and theming support.
- <div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]">
+ <div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[var(--bullet-gradient-from,#FF9966)] to-[var(--bullet-gradient-to,#FF8CA9)]">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"> | |
<span className="absolute inset-0 flex items-center justify-center"> | |
<span className="py-0.25 rounded-full px-0.5 text-sm leading-none text-white"> | |
{number} | |
</span> | |
</span> | |
</div> | |
<div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[var(--bullet-gradient-from,#FF9966)] to-[var(--bullet-gradient-to,#FF8CA9)]"> | |
<span className="absolute inset-0 flex items-center justify-center"> | |
<span className="py-0.25 rounded-full px-0.5 text-sm leading-none text-white"> | |
{number} | |
</span> | |
</span> | |
</div> |
const BulletedNumber: React.FC<BulletedNumberProps> = ({ | ||
number, | ||
current = false, | ||
}) => | ||
current ? ( | ||
<div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center rounded-full border border-black opacity-50"> | ||
<span className="absolute inset-0 flex items-center justify-center"> | ||
<span className="py-0.25 rounded-full bg-white px-0.5 text-sm leading-none text-black"> | ||
{number} | ||
</span> | ||
</span> | ||
</div> | ||
) : ( | ||
<div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"> | ||
<span className="absolute inset-0 flex items-center justify-center"> | ||
<span className="py-0.25 rounded-full px-0.5 text-sm leading-none text-white"> | ||
{number} | ||
</span> | ||
</span> | ||
</div> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add accessibility improvements for screen readers.
While the visual implementation is clean, the component could benefit from improved accessibility.
Apply these changes to enhance accessibility:
const BulletedNumber: React.FC<BulletedNumberProps> = ({
number,
current = false,
}) =>
current ? (
<div
+ role="status"
+ aria-label={`Step ${number}${current ? ' (current step)' : ''}`}
className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center rounded-full border border-black opacity-50">
<span className="absolute inset-0 flex items-center justify-center">
<span className="py-0.25 rounded-full bg-white px-0.5 text-sm leading-none text-black">
{number}
</span>
</span>
</div>
) : (
<div
+ role="status"
+ aria-label={`Step ${number}`}
className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const BulletedNumber: React.FC<BulletedNumberProps> = ({ | |
number, | |
current = false, | |
}) => | |
current ? ( | |
<div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center rounded-full border border-black opacity-50"> | |
<span className="absolute inset-0 flex items-center justify-center"> | |
<span className="py-0.25 rounded-full bg-white px-0.5 text-sm leading-none text-black"> | |
{number} | |
</span> | |
</span> | |
</div> | |
) : ( | |
<div className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"> | |
<span className="absolute inset-0 flex items-center justify-center"> | |
<span className="py-0.25 rounded-full px-0.5 text-sm leading-none text-white"> | |
{number} | |
</span> | |
</span> | |
</div> | |
); | |
const BulletedNumber: React.FC<BulletedNumberProps> = ({ | |
number, | |
current = false, | |
}) => | |
current ? ( | |
<div | |
role="status" | |
aria-label={`Step ${number}${current ? ' (current step)' : ''}`} | |
className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center rounded-full border border-black opacity-50"> | |
<span className="absolute inset-0 flex items-center justify-center"> | |
<span className="py-0.25 rounded-full bg-white px-0.5 text-sm leading-none text-black"> | |
{number} | |
</span> | |
</span> | |
</div> | |
) : ( | |
<div | |
role="status" | |
aria-label={`Step ${number}`} | |
className="relative mt-2 flex h-6 w-6 flex-shrink-0 items-center justify-center overflow-hidden rounded-full bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"> | |
<span className="absolute inset-0 flex items-center justify-center"> | |
<span className="py-0.25 rounded-full px-0.5 text-sm leading-none text-white"> | |
{number} | |
</span> | |
</span> | |
</div> | |
); |
<div className="gradient-border relative overflow-hidden"> | ||
<div className="absolute inset-0 bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"></div> | ||
<div className="absolute inset-0 border-2 border-solid border-transparent"></div> | ||
<div className="mb-1"></div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider simplifying gradient border implementation
The gradient border implementation could be simplified using a single div with border-image.
-<div className="gradient-border relative overflow-hidden">
- <div className="absolute inset-0 bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"></div>
- <div className="absolute inset-0 border-2 border-solid border-transparent"></div>
- <div className="mb-1"></div>
-</div>
+<div className="h-1 w-full border-2 border-solid" style={{
+ borderImage: 'linear-gradient(to right, #FF9966, #FF8CA9) 1'
+}}></div>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div className="gradient-border relative overflow-hidden"> | |
<div className="absolute inset-0 bg-gradient-to-r from-[#FF9966] to-[#FF8CA9]"></div> | |
<div className="absolute inset-0 border-2 border-solid border-transparent"></div> | |
<div className="mb-1"></div> | |
</div> | |
<div className="h-1 w-full border-2 border-solid" style={{ | |
borderImage: 'linear-gradient(to right, #FF9966, #FF8CA9) 1' | |
}}></div> |
<div className="flex items-center"> | ||
<SideFunding | ||
side={SideEnum.claimer} | ||
disputeId={disputeId} | ||
arbitrator={arbitrator!} | ||
contributor={contributor} | ||
requester={claimer} | ||
requesterFunds={claimerFunds} | ||
appealCost={totalClaimerCost} | ||
chainId={chainId} | ||
loosingSideHasEnd={ | ||
currentRulingFormatted === SideEnum.challenger | ||
? loosingSideHasEnd | ||
: false | ||
} | ||
/> | ||
<SideFunding | ||
side={SideEnum.challenger} | ||
disputeId={disputeId} | ||
arbitrator={arbitrator!} | ||
contributor={contributor} | ||
requester={challenger} | ||
requesterFunds={challengerFunds} | ||
appealCost={totalChallengerCost} | ||
chainId={chainId} | ||
loosingSideHasEnd={ | ||
currentRulingFormatted === SideEnum.claimer | ||
? loosingSideHasEnd | ||
: false | ||
} | ||
/> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simplify funding section conditional logic
The nested ternary operators for loosingSideHasEnd
could be simplified using a function.
+const isLoosingSide = (side: SideEnum, currentRuling: SideEnum) => {
+ return (side === SideEnum.claimer && currentRuling === SideEnum.challenger) ||
+ (side === SideEnum.challenger && currentRuling === SideEnum.claimer);
+};
<SideFunding
// ... other props
- loosingSideHasEnd={
- currentRulingFormatted === SideEnum.challenger
- ? loosingSideHasEnd
- : false
- }
+ loosingSideHasEnd={isLoosingSide(SideEnum.claimer, currentRulingFormatted) && loosingSideHasEnd}
/>
Committable suggestion skipped: line range outside the PR's diff.
<Image | ||
alt="warning" | ||
src="/logo/exclamation.svg" | ||
height={24} | ||
width={24} | ||
/> | ||
Policy in force at submission | ||
<div className="outline-color: #E5E5E5 bg-whiteBackground text-secondaryText invisible absolute left-1/2 z-10 m-4 mx-auto w-[260px] flex-shrink-0 -translate-x-1/2 transform place-content-center content-between rounded-[3px] border-[1px] border-[solid] bg-[var(--Light-Mode-White-background,_#FFF)] p-[8px] text-justify text-[14px] font-normal not-italic leading-[normal] outline-black transition-opacity ease-in-out [box-shadow:0px_2px_3px_0px_rgba(0,_0,_0,_0.06)] group-hover:visible md:w-[400px]"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider improving tooltip accessibility and readability.
While the warning icon and tooltip provide valuable context about policy versioning, consider these improvements:
- The tooltip's visibility relies on CSS hover, which isn't touch-device friendly.
- The tooltip text is quite dense and could benefit from better formatting.
Consider these enhancements:
-<div className="text-primaryText group relative flex py-[8px]">
+<div className="text-primaryText group relative flex py-[8px]"
+ role="button"
+ tabIndex={0}
+ onClick={() => setTooltipVisible(!tooltipVisible)}
+ onKeyPress={(e) => e.key === 'Enter' && setTooltipVisible(!tooltipVisible)}>
<Image
alt="warning"
src="/logo/exclamation.svg"
height={24}
width={24}
/>
Policy in force at submission
- <div className="outline-color: #E5E5E5 bg-whiteBackground text-secondaryText invisible absolute left-1/2 z-10 m-4 mx-auto w-[260px] flex-shrink-0 -translate-x-1/2 transform place-content-center content-between rounded-[3px] border-[1px] border-[solid] bg-[var(--Light-Mode-White-background,_#FFF)] p-[8px] text-justify text-[14px] font-normal not-italic leading-[normal] outline-black transition-opacity ease-in-out [box-shadow:0px_2px_3px_0px_rgba(0,_0,_0,_0.06)] group-hover:visible md:w-[400px]">
+ <div className={`outline-color: #E5E5E5 bg-whiteBackground text-secondaryText absolute left-1/2 z-10 m-4 mx-auto w-[260px] flex-shrink-0 -translate-x-1/2 transform place-content-center content-between rounded-[3px] border-[1px] border-[solid] bg-[var(--Light-Mode-White-background,_#FFF)] p-[8px] text-[14px] font-normal not-italic leading-[normal] outline-black transition-opacity ease-in-out [box-shadow:0px_2px_3px_0px_rgba(0,_0,_0,_0.06)] md:w-[400px] ${tooltipVisible ? 'visible' : 'invisible'}`}
+ role="tooltip">
<span>
- This is the policy that was in effect when this submission was made. Why is this important? Policies may change over time, and it's crucial to know the policy that was in force at the time of a submission before challenging or removing a profile. If you challenge this submission, this version of the policy will be enforced by Kleros jurors if the case goes to arbitration. Also, if you revoke this profile citing "incorrect submission," but the submission complied with this policy, your revocation request may be rejected, and you may lose your deposit.
+ <p className="mb-2"><strong>Why is this important?</strong></p>
+ <p className="mb-2">This is the policy that was in effect when this submission was made. Policies may change over time, and it's crucial to know the applicable policy before challenging or removing a profile.</p>
+ <p className="mb-2">If you challenge this submission, this version of the policy will be enforced by Kleros jurors during arbitration.</p>
+ <p>Note: Revoking this profile for "incorrect submission" when it complied with this policy may result in rejection and deposit loss.</p>
</span>
</div>
</div>
You'll need to add state management at the component level:
const [tooltipVisible, setTooltipVisible] = useState(false);
Summary by CodeRabbit
Release Notes
New Features
ActionBar
andAppeal
components to support new properties for improved user interaction.BulletedNumber
component for better visual representation of numbered steps.UI Improvements
Appeal
component, replacing theAccordion
with a more straightforward design.Info
component to display customizable labels and improved hover effects.Request
component.Bug Fixes